Skip to content

Fan out display-name rename to every cohort (ARB-513)#390

Merged
crthpl merged 1 commit intomainfrom
crthpl/arb-513-display-name-rename
Apr 13, 2026
Merged

Fan out display-name rename to every cohort (ARB-513)#390
crthpl merged 1 commit intomainfrom
crthpl/arb-513-display-name-rename

Conversation

@crthpl
Copy link
Copy Markdown
Contributor

@crthpl crthpl commented Apr 13, 2026

Summary

  • Renaming fans out to every cohort. Updating your display name now propagates global_user.display_name plus every non-read-only cohort's account.name in one shot, via a shared apply_user_rename helper used by both PUT /api/users/me/display-name and PUT /api/admin/users/:id/display-name.
  • Per-cohort -N collision suffixes. When a cohort already has an account with the target name, the backend picks the smallest available -2/-3/… suffix. The old -g{global_user_id} scheme in ensure_user_created_by_global_id is gone.
  • Preflight + confirmation flow. Before any writes, the server computes the target name in each cohort. If any would need a suffix, it responds 409 { status: 'needs_confirmation', conflicts: [...] } and writes nothing. The frontend opens an AlertDialog listing the suggested per-cohort names; on confirm it resubmits with a confirmed_overrides map. A TOCTOU race during commit bubbles back out as a plain 409 — the client retries from scratch.
  • Same-name requests rejected with 400 "Display name unchanged", so a double-click doesn't ratchet suffixes.
  • Live broadcast. After each cohort rename, the server sends a single-entry SM::Accounts { accounts: [updated] } via cohort.subscriptions.send_public(...). The frontend handler for msg.accounts had a latent bug (it cleared the whole map before looping, so single-element broadcasts wiped out state); fixed to upsert. Full resets still happen through resetServerState() on disconnect.
  • Accounts page layout — shows a single Name: Theo Reinsberg line when the current cohort's account name matches the global name, and two lines (Name for {cohort}: Theo Reinsberg-2 + Name: Theo Reinsberg) when they diverge. Only the global name is editable; the edit prefills from the global name, not the suffixed cohort-local one.
  • New GET /api/users/me returning { id, display_name, email, is_admin } for the caller, so the accounts page can show the global name even when it differs from the current cohort's account.name.
  • Admin rename goes through the same preflight/commit flow with its own confirmation modal on the admin page.
  • Out of scope: historical -g{N}-style cohort account names from earlier code paths aren't rewritten; a subsequent rename clears them out naturally. Also removes a stale top-level DEPLOY.md that was already unused.

Fixes ARB-513

Test plan

  • cd backend && cargo clippy --features dev-mode --tests -- -D warnings — no warnings (verified locally)
  • cd backend && cargo test-all — all 62 tests pass (verified locally)
  • cd frontend && pnpm run check && pnpm run lint — 0 errors (verified locally)
  • Manual: log in as Alice in cohort main, rename her to "Bob" (no collision). Expect single Name: Bob line, other connected clients see the rename live.
  • Manual: create a second account named "Bob" in cohort willow, then as Alice submit a rename to "Bob". Expect the confirmation modal listing Willow: Bob-2. Confirm → main shows Name: Bob, willow shows two lines (Name for willow: Bob-2 / Name: Bob).
  • Manual: submit the same name twice (BobBob) — expect 400 "Display name unchanged".
  • Manual: two-tab simultaneous rename to the same target to exercise the TOCTOU 409 path.
  • Manual (admin page): rename another user with a deliberate collision → admin sees the parallel confirmation modal and the rename applies across their cohorts.

…llisions (ARB-513)

Renaming a user's global display name now propagates to every non-read-only
cohort they're in. Collisions with other accounts in a cohort are resolved by
appending the smallest available `-N` suffix, surfaced to the user through a
preflight/confirmation flow so they consent to any suffix before it's applied.

Fixes ARB-513
@crthpl crthpl requested a review from a team as a code owner April 13, 2026 23:19
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
platform Ready Ready Preview, Comment Apr 13, 2026 11:20pm

Request Review

@crthpl crthpl merged commit c5f4008 into main Apr 13, 2026
5 checks passed
briansmiley added a commit that referenced this pull request Apr 14, 2026
Reverts commits 7cd390c..c5f4008 (15 commits) to restore the
pre-multi-cohort state. Critical event tomorrow — rolling back to
last known stable state.

Reverted commits:
- 7cd390c Multi-cohort support with per-member initial balance (#358)
- 1a9259e Allow sudoed admins higher decimal precision (#370)
- 612f7e7 Add select all / clear buttons to transfer recipient multiselect (#350)
- ec96705 Link cohort members to existing users when added by email (#378)
- eed7a73 Consolidate admin page data loading into single /api/admin/overview (#381)
- eac92d0 Populate global user display name from id_token on login (ARB-515) (#380)
- c5461ee Remove redundant Refresh button from /admin page (#382)
- 77168e3 Sync Kinde admin role into global_user.is_admin (ARB-512) (#379)
- 685ee0c Remove dead get_market_positions and fix 0.5.0 ImportError (#385)
- e9d49f7 Constrain initial balance inputs to numeric values (#383)
- 95ff300 Stop overwriting global_user display_name on admin REST calls (#386)
- 0163100 Show user email on admin page while editing display name (#387)
- 85f2c5f Fix user selection dropdown hover when multiple users have same name (#388)
- 3fedb38 Add admins to cohort member list on access (ARB-510) (#389)
- c5f4008 Fan out display-name rename to every cohort (ARB-513) (#390)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
briansmiley added a commit that referenced this pull request Apr 14, 2026
Reverts commits 7cd390c..c5f4008 (15 commits) to restore the
pre-multi-cohort state. Critical event tomorrow — rolling back to
last known stable state.

Reverted commits:
- 7cd390c Multi-cohort support with per-member initial balance (#358)
- 1a9259e Allow sudoed admins higher decimal precision (#370)
- 612f7e7 Add select all / clear buttons to transfer recipient multiselect (#350)
- ec96705 Link cohort members to existing users when added by email (#378)
- eed7a73 Consolidate admin page data loading into single /api/admin/overview (#381)
- eac92d0 Populate global user display name from id_token on login (ARB-515) (#380)
- c5461ee Remove redundant Refresh button from /admin page (#382)
- 77168e3 Sync Kinde admin role into global_user.is_admin (ARB-512) (#379)
- 685ee0c Remove dead get_market_positions and fix 0.5.0 ImportError (#385)
- e9d49f7 Constrain initial balance inputs to numeric values (#383)
- 95ff300 Stop overwriting global_user display_name on admin REST calls (#386)
- 0163100 Show user email on admin page while editing display name (#387)
- 85f2c5f Fix user selection dropdown hover when multiple users have same name (#388)
- 3fedb38 Add admins to cohort member list on access (ARB-510) (#389)
- c5f4008 Fan out display-name rename to every cohort (ARB-513) (#390)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant